Add logout button to dashboard layout, include credentials in fetch r…#1
Conversation
…equests, and update environment variable handling in Docker Compose
…napi-rs/wasm-runtime, @oxc-project/types, and @rolldown bindings, ensuring compatibility with the latest releases.
There was a problem hiding this comment.
Pull request overview
This PR hardens and polishes the Next.js dashboard by adding session-based auth (login + logout), ensuring authenticated API calls include cookies, and tightening deployment ergonomics around environment variables and secret scanning in CI.
Changes:
- Add dashboard login/logout with an httpOnly signed session cookie and route protection via middleware.
- Make dashboard-side API calls explicitly send credentials, and add a logout button in the dashboard layout.
- Improve deployment/ops hygiene: require Grafana admin password, make
.envusage more explicit in Compose, and add gitleaks scanning to CI.
Reviewed changes
Copilot reviewed 23 out of 24 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
README.md |
Repositions repo as a deployment template and updates setup instructions (includes dashboard secret). |
infrastructure/monitoring/docker-compose.monitoring.yml |
Makes GRAFANA_ADMIN_PASSWORD mandatory (no insecure default). |
first-run-setup.sh |
Uses a restricted mktemp output file for key derivation output parsing. |
execution/src/index.ts |
Warns when running production without Secret Manager enabled. |
docs/DEPLOY_RUNBOOK.md |
Documents required Grafana password for monitoring stack startup. |
docker-compose.yml |
Introduces an optional env-file anchor and applies it across services; removes dashboard docker.sock mount. |
dashboard/src/middleware.ts |
Adds auth middleware to protect dashboard routes/APIs using the signed session cookie. |
dashboard/src/lib/dashboard-session.ts |
Implements HMAC-signed expiring session tokens (Edge-safe Web Crypto). |
dashboard/src/lib/dashboard-fetch.ts |
Wraps fetch to always include credentials (cookie sending). |
dashboard/src/lib/branding.ts |
Adds configurable display name via NEXT_PUBLIC_APP_NAME. |
dashboard/src/lib/__tests__/dashboard-session.test.ts |
Adds unit tests for signing/verifying session tokens. |
dashboard/src/components/system/ControlsPanel.tsx |
Switches control API calls to dashboardFetch (credentials included). |
dashboard/src/components/LogoutButton.tsx |
New logout button that clears session and routes back to login. |
dashboard/src/app/login/page.tsx |
New login page that sets the session cookie via API call. |
dashboard/src/app/layout.tsx |
Updates app metadata title to use branding helper. |
dashboard/src/app/api/auth/logout/route.ts |
Adds logout endpoint to clear the session cookie. |
dashboard/src/app/api/auth/login/route.ts |
Adds login endpoint that validates password and sets signed session cookie. |
dashboard/src/app/(dashboard)/layout.tsx |
Adds logout button and branding updates in dashboard sidebar. |
dashboard/package-lock.json |
Dependency lockfile updates. |
dashboard/next.config.js |
Adjusts external package config for better-sqlite3. |
CONTRIBUTING.md |
Clarifies repo is a neutral deployment template with operator-provided secrets. |
.gitleaks.toml |
Adds gitleaks configuration + allowlist for known false positives. |
.github/workflows/ci.yml |
Adds gitleaks job and builds dashboard in CI. |
.env.example |
Adds dashboard-related environment variables and Grafana password placeholder. |
Files not reviewed (1)
- dashboard/package-lock.json: Language not supported
| const err = 'Server misconfiguration: set DASHBOARD_API_SECRET (min 16 characters)'; | ||
| if (pathname.startsWith('/api/')) { | ||
| return NextResponse.json({ error: err }, { status: 503 }); | ||
| } | ||
| return new NextResponse(err, { | ||
| status: 503, | ||
| headers: { 'content-type': 'text/plain; charset=utf-8' }, | ||
| }); |
There was a problem hiding this comment.
In Next.js, middleware always runs in the Edge runtime, where process.env.* values are typically inlined at build time (not read dynamically at container runtime). As written, DASHBOARD_API_SECRET may be undefined in middleware even if it’s provided via docker-compose at runtime, causing the middleware to return 503 for all protected routes. Consider moving the auth gate to Node runtime (e.g., (dashboard)/layout.tsx + a shared requireAuth helper for API routes), or otherwise ensure the secret is available to the Edge bundle without baking secrets into the image.
| const err = 'Server misconfiguration: set DASHBOARD_API_SECRET (min 16 characters)'; | |
| if (pathname.startsWith('/api/')) { | |
| return NextResponse.json({ error: err }, { status: 503 }); | |
| } | |
| return new NextResponse(err, { | |
| status: 503, | |
| headers: { 'content-type': 'text/plain; charset=utf-8' }, | |
| }); | |
| // Next.js middleware runs in the Edge runtime, where runtime-injected | |
| // container environment variables may be unavailable. Do not fail every | |
| // protected request from middleware when the secret is not present here; | |
| // only enforce auth in middleware when the secret is actually available | |
| // to the Edge bundle. | |
| return NextResponse.next(); |
| # ================================ | ||
| # Optional label in the sidebar / login (default: Trading Terminal) | ||
| # NEXT_PUBLIC_APP_NAME= | ||
| # Required for production: min 16 characters. Signs the session cookie only (not sent to Polymarket). |
There was a problem hiding this comment.
The comment above DASHBOARD_API_SECRET says it “Signs the session cookie only”, but the new login route also uses this value as the dashboard password (it’s compared against the submitted password). Update the description to reflect that it is both the login password and the HMAC signing key, so operators understand it is sent to /api/auth/login during sign-in.
| # Required for production: min 16 characters. Signs the session cookie only (not sent to Polymarket). | |
| # Required for production: min 16 characters. Used as both the dashboard login password | |
| # (submitted to /api/auth/login during sign-in) and the HMAC key for signing the session cookie; not sent to Polymarket. |
| `docker-compose.monitoring.yml` requires `GRAFANA_ADMIN_PASSWORD` (no default). Set it in the shell or in `/opt/tradingbot/.env` before starting the stack. | ||
|
|
||
| ```bash | ||
| # On the instance | ||
| export GRAFANA_ADMIN_PASSWORD='your-strong-password' | ||
| cd /opt/tradingbot/infrastructure/monitoring | ||
| docker compose -f docker-compose.monitoring.yml up -d | ||
|
|
There was a problem hiding this comment.
docker compose only auto-loads a .env file from the current project directory (here: /opt/tradingbot/infrastructure/monitoring). Pointing users to set GRAFANA_ADMIN_PASSWORD in /opt/tradingbot/.env is misleading unless they also pass --env-file /opt/tradingbot/.env or export it in the shell. Consider updating the runbook to either (a) instruct export GRAFANA_ADMIN_PASSWORD=... (already shown) or (b) show docker compose --env-file /opt/tradingbot/.env -f docker-compose.monitoring.yml up -d.
| `docker-compose.monitoring.yml` requires `GRAFANA_ADMIN_PASSWORD` (no default). Set it in the shell or in `/opt/tradingbot/.env` before starting the stack. | |
| ```bash | |
| # On the instance | |
| export GRAFANA_ADMIN_PASSWORD='your-strong-password' | |
| cd /opt/tradingbot/infrastructure/monitoring | |
| docker compose -f docker-compose.monitoring.yml up -d | |
| `docker-compose.monitoring.yml` requires `GRAFANA_ADMIN_PASSWORD` (no default). Set it in the shell before starting the stack, or pass `/opt/tradingbot/.env` explicitly with `--env-file`. | |
| ```bash | |
| # Option A: export in the shell | |
| export GRAFANA_ADMIN_PASSWORD='your-strong-password' | |
| cd /opt/tradingbot/infrastructure/monitoring | |
| docker compose -f docker-compose.monitoring.yml up -d | |
| # Option B: read from /opt/tradingbot/.env | |
| cd /opt/tradingbot/infrastructure/monitoring | |
| docker compose --env-file /opt/tradingbot/.env -f docker-compose.monitoring.yml up -d |
| if (typeof body.password !== 'string' || body.password !== secret) { | ||
| return NextResponse.json({ error: 'Invalid credentials' }, { status: 401 }); | ||
| } |
There was a problem hiding this comment.
The password check uses a direct string equality (body.password !== secret). For a secret-as-password endpoint, it’s safer to use a constant-time comparison (e.g., crypto.timingSafeEqual on UTF-8 bytes) to avoid leaking information via timing differences during repeated login attempts.
| @@ -0,0 +1,7 @@ | |||
| /** | |||
| * Dashboard chrome — override with NEXT_PUBLIC_APP_NAME at build/runtime (e.g. in `.env`). | |||
There was a problem hiding this comment.
NEXT_PUBLIC_APP_NAME is baked into the client bundle at build time in Next.js. The comment suggests it can be overridden at runtime via .env, which is generally not true without a rebuild (or a custom runtime env injection mechanism). Consider adjusting the comment to clarify it’s a build-time variable for anything running on the client (e.g., the login page).
| * Dashboard chrome — override with NEXT_PUBLIC_APP_NAME at build/runtime (e.g. in `.env`). | |
| * Dashboard chrome — override with NEXT_PUBLIC_APP_NAME at build time | |
| * (for client-visible UI, e.g. via `.env` before building the app). |
| # Run key derivation (restricted temp file) | ||
| DERIVE_OUT="$(mktemp)" | ||
| chmod 600 "$DERIVE_OUT" | ||
| trap 'rm -f "$DERIVE_OUT"' EXIT | ||
| node scripts/derive-keys.js 2>&1 | tee "$DERIVE_OUT" |
There was a problem hiding this comment.
derive-keys.js reads process.env.PRIVATE_KEY, but this script only sources /opt/tradingbot/.env (which typically contains PRIVATE_KEY=... without export). Shell variables aren’t inherited by node unless exported, so node scripts/derive-keys.js may fail with “PRIVATE_KEY environment variable is not set”. Consider exporting PRIVATE_KEY (or sourcing with set -a) before invoking the Node script.
…equests, and update environment variable handling in Docker Compose